Skip to content

Bump OpenIddict to 6.1.1#17582

Merged
kevinchalet merged 2 commits intoOrchardCMS:mainfrom
kevinchalet:openiddict_6.1.1
Mar 9, 2025
Merged

Bump OpenIddict to 6.1.1#17582
kevinchalet merged 2 commits intoOrchardCMS:mainfrom
kevinchalet:openiddict_6.1.1

Conversation

@kevinchalet
Copy link
Member

No description provided.

@kevinchalet kevinchalet added OpenId dependencies Pull requests that update a dependency file labels Mar 9, 2025
@kevinchalet kevinchalet added this to the 3.0 milestone Mar 9, 2025
@kevinchalet kevinchalet self-assigned this Mar 9, 2025

<!-- dotnet/extensions repository -->
<PackageVersion Include="Microsoft.Extensions.Http.Resilience" Version="9.1.0" />
<PackageVersion Include="Microsoft.Extensions.Http.Resilience" Version="9.2.0" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: bumping that dependency was required as OpenIddict references the 9.2.0 version of that package.

// Note: caching is enabled for both authorization and end session requests to allow sending
// large POST authorization and end session requests, but can be programmatically disabled, as the
// authorization and end session views support flowing the entire payload and not just the request_uri.
options.EnableAuthorizationRequestCaching = true;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: these options were moved to OpenIddictServerOptions (instead of OpenIddictServerAspNetCoreOptions) as part of the OAuth 2.0 Pushed Authorization Requests introduction. The old properties are still there but obsolete and no-op.

return View(new AuthorizeViewModel
{
ApplicationName = await _applicationManager.GetLocalizedDisplayNameAsync(application),
RequestId = request.RequestId,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: OpenIddictRequest.RequestId is obsolete in OpenIddict 6.1.0+ (the request caching feature now uses request_uri) but that property wasn't used anywhere so I decided to remove it.

authorization => authorization.CreationDate < threshold.UtcDateTime &&
(authorization.Status != OpenIddictConstants.Statuses.Valid ||
(authorization.Type == OpenIddictConstants.AuthorizationTypes.AdHoc &&
(authorization.Status != Statuses.Valid || authorization.Type == AuthorizationTypes.AdHoc) &&
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: that behavior change is deliberate: openiddict/openiddict-core#2251.

@Piedone Piedone mentioned this pull request Mar 9, 2025
1 task
Copy link
Member

@Piedone Piedone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update was in the Renovate queue as well. I now triggered it: #17583. Could you please check if that's right (as far as automatic updates can go, like the one-off Microsoft.Extensions.Http.Resilience fluke can't be fixed by Renovate, but perhaps we need to update it in the same group?), and if not, change the config here: https://github.com/OrchardCMS/OrchardCore/blob/main/renovate.json5#L62

@kevinchalet
Copy link
Member Author

kevinchalet commented Mar 9, 2025

The bot config looks good. I made my tests without bumping the two other dependencies, but if you prefer, I can bump them as part of this PR and close the automatic one.

Regarding Microsoft.Extensions.Http.Resilience, we could do that, but if we end up doing the same thing for all the packages OpenIddict itself references, the list may be long. Let's wait until we think we really need that?

Copy link
Member

@Piedone Piedone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bot config looks good. I made my tests without bumping the two other dependencies, but if you prefer, I can bump them as part of this PR and close the automatic one.

Let's bump those too, then. Please merge from that PR and then we can merge this (after which that'll be closed automatically).

Regarding Microsoft.Extensions.Http.Resilience, we could do that, but if we end up doing for all the packages OpenIddict itself references, the list may be long. Let's wait until we think we really need that?

OK, let's leave it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file OpenId

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants